300207 - Scrollbars in ClearLooks theme do not have rounded corners

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Randy Eckenrode]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b3) Gecko/20050709 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b3) Gecko/20050709 Firefox/1.0+

ClearLooks draws rounded corners on its scrollbar buttons. It determines whether
to draw left/top buttons or right/bottom ones by examing the allocation member
of the GtkScrollbar it is drawing. Because the scrollbars passed to gtk_draw_box
have their default allocations, this test always fails and the default,
non-rounded box is drawn.

Reproducible: Always

Steps to Reproduce:
1. Run Firefox/GTK2 with ClearLooks as the theme
2. Examine the scrollbars

Actual Results:  
The scrollbars have sharp corners.

Expected Results:  
The scrollbars should have rounded corners.

Comment 1

[Randy Eckenrode]

Attachment: Attempt at fixing the problem

This modifies the allocation so that the ClearLooks theme draws correctly. It
doesn't appear to break any of the other GTK+ themes I've tried, but it does
modify the global scrollbar variables, which might be bad (I'm just getting
aquainted with the source).

Comment 2

[Randy Eckenrode]

(In reply to comment #1)
Oh, and yes this is rather hackish. :-)

Comment 3

[Randy Eckenrode]

Attachment: Less hacky scrollbar patch

Create an allocation for the scrollbar to please the themes that care

Comment 4

[Adam Guthrie]

Similar to bug 206554. You need to request a review from someone on your patch
for anything to happen. Either roc or pav would probably be suitable here.

Comment 5

[Randy Eckenrode]

Attachment: Respin of previous patch

This is a respin of the previous patch against HEAD that was sent to me.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I checked this in.

Comment 7

[Jonathan Haas]

The up and left arrows are rounded, but the down and right arrows are still stupid rectangles.

Maybe this is because I use the following UserChrome.css-rule to make the additional up/left-arrow visible, but I didn't tested it without them. 

scrollbarbutton[sbattr="scrollbar-up-bottom"] {display: -moz-box !important;}

Comment 8

[Jonathan Haas]

The bug appears also without the UserChrome-Code. Up- and left-arrows work fine, but down and right-arrows are still rectangular.

Comment 9

[Adam Guthrie]

Comment on attachment 204488 [details] [diff] [review]
Respin of previous patch

Low-risk patch, baked on trunk, and would be good UI polish for branch.

Comment 10

[Jonathan Haas]

Please look at Bug 336270 first.

No rounded corners may be better than incorrectly rounded corners. IMHO.

Comment 11

[Randy Eckenrode]

Attachment: Scrollbar patch mk3

This one should fix the problem with the right/bottom steppers not being rounded. Double steppers (e.g. in Clearlooks-NeXT) don’t renderer properly (the inner ones should not be rounded), but it’s not immediately obvious whether one can know if he should drawn an inner stepper or not. I’m also not sure why using three times the stepper size for the fake allocation works but double doesn’t; maybe someone better versed in the peculiarities of GTK+ would know.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, I just figured out independently that using triple the button size worked but double (as the current code does) doesn't.

That said, I think the patch belongs in bug 336270.

What's the difference between what button_rect and *rect represent ?  In my build, they're always the same, and the code doesn't really help me understand what they're supposed to be.  Did which one you used make a difference for you?

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(My gut feeling is that the code using rect is probably more likely to be correct.)

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

...followup discussion should probably go on bug 336270; putting things in already-fixed bugs is a good way to get them lost.